Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assertions refactoring #1566

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

robsunday
Copy link
Contributor

@robsunday robsunday commented Nov 22, 2024

Preview of new metrics assertions framework.
Assertion errors are now reported in much more compact and descriptive form.
Error message always contain only data related to failing metric, as opposed to old assertions where all received metrics were listed and it was extremely hard to find what is wrong in large set of metrics.

Copy link
Contributor Author

@robsunday robsunday Nov 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[question to reviewers] In original framework we had assertSum method with overloaded version accepting isMonotonic parameter added. This was little unclear to me for two reasons:

It was not obvious that assertSum without isMonotonic parameter was the same as assertSum(isMonotonic=true)
In YAML we used instrument types gauge, counter, updowncounter, while in assertions we used assertGauge, assertSum with and without isMonotonic parameter.
That's why I decided to implement assertGauge, assertCounter and assertUpDownCounter method families here.
I know it may lead to more methods to be implemented but for me this new API is cleaner.

What do you think about this approach?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants